-
Notifications
You must be signed in to change notification settings - Fork 237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Problem: memiavl snapshot format is not optimal #890
Conversation
@@ -148,7 +148,7 @@ | |||
if _, err := w.Write(buf[0:n]); err != nil { | |||
return fmt.Errorf("writing size, %w", err) | |||
} | |||
n = binary.PutVarint(buf[:], node.Version()) | |||
n = binary.PutVarint(buf[:], int64(node.Version())) |
Check failure
Code scanning / gosec
Potential integer overflow by integer type conversion
@@ -283,7 +283,7 @@ | |||
if _, err := w.Write(buf[:n]); err != nil { | |||
return err | |||
} | |||
n = binary.PutVarint(buf[:], node.Version()) | |||
n = binary.PutVarint(buf[:], int64(node.Version())) |
Check failure
Code scanning / gosec
Potential integer overflow by integer type conversion
Codecov Report
@@ Coverage Diff @@
## main #890 +/- ##
==========================================
+ Coverage 36.60% 36.78% +0.18%
==========================================
Files 56 57 +1
Lines 4065 4170 +105
==========================================
+ Hits 1488 1534 +46
- Misses 2406 2460 +54
- Partials 171 176 +5
|
} | ||
|
||
// Version returns the current tree version | ||
func (t *Tree) Version() int64 { | ||
return t.version | ||
return int64(t.version) |
Check failure
Code scanning / gosec
Potential integer overflow by integer type conversion
@@ -66,12 +78,12 @@ | |||
t.version = t.initialVersion | |||
} | |||
|
|||
return hash, t.version, nil | |||
return hash, int64(t.version), nil |
Check failure
Code scanning / gosec
Potential integer overflow by integer type conversion
tree := New() | ||
tree.initialVersion = initialVersion | ||
tree.initialVersion = uint32(initialVersion) |
Check failure
Code scanning / gosec
Potential integer overflow by integer type conversion
if version >= math.MaxUint32 { | ||
panic("version overflows uint32") | ||
} | ||
return &Tree{version: uint32(version)} |
Check failure
Code scanning / gosec
Potential integer overflow by integer type conversion
Solution: - store offset table inside keys/values file, so we can reference it with leaf index in nodes. - leverage the property of post-order traversal to further reduce some offsets. - specialize `Get` operation for persisted node to improve query performance. - add native-endian implementation to improve performance. in the end, the on-disk IAVL tree query performance is on-par with tidwall/btree library which is used for cache store.
Signed-off-by: yihuang <huang@crypto.com>
return err | ||
} | ||
len2 := uint64(binary.LittleEndian.Uint32(numBuf[:])) | ||
if _, err := io.CopyN(io.Discard, reader, int64(len2)); err != nil { |
Check failure
Code scanning / gosec
Potential integer overflow by integer type conversion
if err := fpNodes.Sync(); err != nil { | ||
|
||
// re-open kvs file for reading | ||
input, err := os.Open(kvsFile) |
Check failure
Code scanning / gosec
Potential file inclusion via variable
return err | ||
} | ||
defer input.Close() |
Check failure
Code scanning / gosec
Deferring unsafe method "Close" on type "*os.File"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Signed-off-by: yihuang <huang@crypto.com>
Solution:
Get
operation for persisted node to improve performance.in the end, the on-disk IAVL tree query performance is on-par with tidwall/btree library which is used for cache store.
Takeaways:
keys
file, which is accessed too frequently, so we use plain little endian int32 array there.👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make
)make test
)go fmt
)golangci-lint run
)go list -json -m all | nancy sleuth
)Thank you for your code, it's appreciated! :)